Disable LTO builds in tests (and bump deps to -O2) #1497
Conversation
TheBlueMatt
commented
May 25, 2022
•
TheBlueMatt
commented
Codecov ReportBase: 87.30% // Head: 90.89% // Increases project coverage by
📣 This organization is not using Codecov’s GitHub App Integration. We recommend you install it so Codecov can continue to function properly for your repositories. Learn more Additional details and impacted files@@ Coverage Diff @@
## main #1497 +/- ##
==========================================
+ Coverage 87.30% 90.89% +3.58%
==========================================
Files 101 77 -24
Lines 44364 42393 -1971
Branches 44364 42393 -1971
==========================================
- Hits 38734 38533 -201
+ Misses 5630 3860 -1770
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. ☔ View full report at Codecov. |
Cargo.toml
Outdated
| # Ideally we would only do this in profile.test, but profile.test only applies to | ||
| # the test binary, not dependencies, which means most of the critical code still | ||
| # gets compiled as -O0. See | ||
| # Our tests do actual crypo and lots of work, the tradeoff for -O1 is well |
There was a problem hiding this comment.
"[...] the tradeoff for -O1 is well worth it."
Wait, I now thought the test and dev profiles are disjunct, and here we're only enabling -O1 for the latter?
Also: s/crypo/crypto
There was a problem hiding this comment.
Yea, so I didn't realize, but test applies for just the crate being tested, dev applies for all the dependencies thereof. I think, but I'm not sure, that we're building the lightning crate in O1 as a dependency for other workspace crates. The change here should make it all-workspace-crates are O0.
|
@wpaulino had some tests that may have indicated this wasn't working as intended (slowed down post-compile |
|
See my results below on a M1 MacBook Pro. Steps before each measurement: I confirmed that At 90372f3 (PR head): At 75ca50f (PR base): |
|
I can confirm this on my development VM (Ubuntu On older server hardware (Ubuntu, |
|
Blocked on rust-lang/rust#97460 |
TheBlueMatt
changed the title
Cargo.toml (and bump deps to -O2)Cargo.toml (and bump deps to -O2)
TheBlueMatt
changed the title
Cargo.toml (and bump deps to -O2)For some reason rustc, at some point, decided that our optimization of dependencies implies we want to also build with LTO. This causes our test builds to take substantially longer to compile, even with only a trivial change. By hard-disabling this (even keeping the optimization of the test and in-tree libraries enabled) the time required to build with only a trivial change to `functional_tests.rs` goes from 0m25.635s wall clock/1m14.220s CPU time to 0m17.841s wall clock/0m17.828s CPU time on my i7-13700K on rustc 1.63. The changes in test execution time appear to be within noise. While we're at it, we also bump dependencies to build with -O2 because their build time is now substantially reduced cost.
TheBlueMatt
force-pushed
the
2022-05-no-default-opt
branch
from
90372f3 to
8d35d2c
Compare
There was a problem hiding this comment.
Without this branch (HEAD^):
- Clean build:
cargo test --no-run 623.52s user 244.74s system 612% cpu 2:21.75 total - Trivial test change build:
cargo test --no-run 189.47s user 135.20s system 500% cpu 1:04.82 total
With this branch:
- Clean build:
cargo test --no-run 551.26s user 87.84s system 610% cpu 1:44.70 total - Trivial test change build:
cargo test --no-run 30.06s user 6.60s system 186% cpu 19.665 total
